-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Separate pybind module from libOpen3D shared library in Python wheels #7355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes. |
- Modified CMakeLists.txt to automatically set BUILD_SHARED_LIBS=ON when BUILD_PYTHON_MODULE=ON - Updated Dockerfile.wheel to remove explicit BUILD_SHARED_LIBS=OFF - Updated Windows workflow to remove explicit BUILD_SHARED_LIBS=OFF for wheel builds - Updated ci_utils.sh to remove BUILD_SHARED_LIBS=OFF from pip package builds This ensures pybind module dynamically links to libOpen3D.so instead of statically embedding it Co-authored-by: ssheorey <[email protected]>
- Removed the NOT BUILD_SHARED_LIBS condition from OpenMP library packaging - Now packages libomp for macOS regardless of build type since Python builds always use shared libs Co-authored-by: ssheorey <[email protected]>
- Removed explicit STATIC_WINDOWS_RUNTIME=ON from Windows wheel workflow - When BUILD_SHARED_LIBS=ON (now automatic for Python), STATIC_WINDOWS_RUNTIME defaults to OFF - Shared libraries should use DLL runtime (/MD) instead of static runtime (/MT) Co-authored-by: ssheorey <[email protected]>
- Added conditional message only when BUILD_SHARED_LIBS was OFF - Clarified why shared libraries are required for Python module Co-authored-by: ssheorey <[email protected]>
|
@copilot Do not change CMakeLists.txt, i.e. allow the option of building python bindings with a static libOpen3D. Instead only change CI, build scripts and docker files to build Python wheels with a shared libOpen3D. However, update the default value of BUILD_SHARED_LIBS=ON, since BUILD_PYTHON_MODULE=ON by default. |
- Reverted the CMakeLists.txt logic that forced BUILD_SHARED_LIBS=ON when BUILD_PYTHON_MODULE=ON - Changed the default value of BUILD_SHARED_LIBS from OFF to ON - This allows users to build Python bindings with static libOpen3D if desired - Updated Dockerfile.wheel comment to reflect the new approach - CI/Docker/build scripts still remove explicit BUILD_SHARED_LIBS=OFF to use the default Co-authored-by: ssheorey <[email protected]>
Done. Changed BUILD_SHARED_LIBS default to ON instead of forcing it. This allows building Python bindings with static libOpen3D if explicitly set to OFF, while CI/Docker/build scripts use the new default (ON) for wheel builds. See commit fe53c51. |
Python wheels currently embed all C++ code into the pybind module via static linking. This PR separates them: pybind module now dynamically links to libOpen3D.so/dylib/dll, both packaged in the wheel.
Changes
Core build logic:
BUILD_SHARED_LIBSfromOFFtoONin CMakeLists.txt-DBUILD_SHARED_LIBS=OFFif neededCI/Docker updates:
BUILD_SHARED_LIBS=OFFfrom:docker/Dockerfile.wheelutil/ci_utils.sh(build_pip_package function).github/workflows/windows.yml(wheel build)STATIC_WINDOWS_RUNTIME=ONfrom Windows wheel build (shared libs require DLL runtime)macOS fix:
NOT BUILD_SHARED_LIBScondition from OpenMP library packaging (previously unhandled)Result
By default, wheels now contain:
Instead of:
Users can still build Python bindings with static libOpen3D by explicitly setting
BUILD_SHARED_LIBS=OFFif needed for custom builds.User code unchanged:
import open3dworks identically.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.